Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prepare code and CI for Python 3.8 #1399

Merged
merged 7 commits into from
Dec 13, 2023
Merged

Prepare code and CI for Python 3.8 #1399

merged 7 commits into from
Dec 13, 2023

Conversation

ricardogsilva
Copy link
Member

@ricardogsilva ricardogsilva commented Nov 11, 2023 •
edited
Loading

Overview

This PR refactors some of the tests (and code) in order to make them pass when running under Python 3.8 and also modifies the main CI workflow to use both Python v3.7 and v3.8

Ideally, it should verify that the pygeoapi is ready to move on to Python 3.8 and drop Python 3.7. This PR does not drop Python 3.7 from CI though - it merely prepares the code for it

Related Issue / Discussion

Additional Information

Some notes about the PR changes:

  • In order to better identify failing tests I have modified some test functions to make use of pytest's parametrize feature. This allows having more granular test cases and helps identify exactly which part of a large test is failing. However, note that I did not change any test logic (if you find some logic that has changed, then it is a bug in this PR)
  • Specifically with regard to the test data file in tests/data/CMC_hrdps_continental_TMP_TGL_80_ps2.5km_2020102700_P005-00.grib2, which is for the gdps-temperature collection in the test configuration, I did change pygeoapi config in order to remove the custom GDAL creation option DATA_ENCODING: COMPLEX_PACKING - the previous value caused a rasterio error under Python 3.8. Additionally, by consulting the data encoding section of the GDAL documentation on the GRIB driver, this option seemed to not be relevant, as the specific file being used as a test sample does not have a NO_DATA value

Contributions and Licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution.
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

@ricardogsilva ricardogsilva marked this pull request as ready for review November 13, 2023 10:58
@ricardogsilva ricardogsilva changed the title Fix tests on Python 3.8 Prepare code and CI for Python 3.8 Nov 13, 2023
@ricardogsilva
Copy link
Member Author

@tomkralidis @kalxas

This PR is part of an effort to drop Python 3.7 from pygeoapi's CI, with the intention of making Python 3.8 the minimum. Can you please review and check whether dropping Py37 is indeed desirable, given that it is now EOL?

tomkralidis
tomkralidis requested changes Nov 15, 2023
View reviewed changes
Copy link
Member

@tomkralidis tomkralidis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ricardogsilva -- see minor comment about pandas. In addition:

  • can we put the pytest changes as part of another PR?
  • +1 to remove 3.7 support as part of this PR

pygeoapi/provider/xarray_edr.py Outdated Show resolved Hide resolved
@ricardogsilva
Copy link
Member Author 

* can we put the pytest changes as part of another PR?

Do you mean the usage of oytest's parametrize? Sure I can do that - I included it here because they made my life easier, as I could isolate the specific test case(s) that were failing and rerun - I can revert to a non-parametrized version if you prefer

@tomkralidis
Copy link
Member 

* can we put the pytest changes as part of another PR?

Do you mean the usage of oytest's parametrize? Sure I can do that - I included it here because they made my life easier, as I could isolate the specific test case(s) that were failing and rerun - I can revert to a non-parametrized version if you prefer

Yes that would work, thanks.

@ricardogsilva
Copy link
Member Author

@tomkralidis

  • Reverted tests back to their previous form
  • Dropped Python 3.7 from CI file
  • Switched pdTimestamp usage to more humble np.datetime64

@tomkralidis tomkralidis merged commit bb2e791 into geopython:master Dec 13, 2023
2 checks passed
@ricardogsilva ricardogsilva mentioned this pull request Dec 13, 2023
Closed
@ricardogsilva ricardogsilva mentioned this pull request Jan 24, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomkralidis tomkralidis tomkralidis approved these changes

@francbartoli francbartoli francbartoli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ricardogsilva @tomkralidis @francbartoli